-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(ci): commit deleted files #692
Conversation
✅ Deploy Preview for api-clients-automation canceled.
|
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :)
Small comment if you want to investigate, otherwise feel free to merge!
@@ -242,7 +242,9 @@ jobs: | |||
run: yarn cli cts run ${{ matrix.client.language }} | |||
|
|||
- name: Zip artifact before storing | |||
run: zip -r -y clients-${{ matrix.client.language }}.zip ${{ matrix.client.path }} ${{ matrix.client.testsToStore }} -x "**/node_modules**" "clients/algoliasearch-client-javascript/.yarn**" "**/.github**" | |||
run: | |||
zip -r -y clients-${{ matrix.client.language }}.zip ${{ matrix.client.path }} ${{ matrix.client.testsToStore }} -x \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the change here only the newline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some folder to ignore, the java and php artifacts were multiple Mb, now less than 500Kb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes that's great!!
@@ -62,7 +62,11 @@ runs: | |||
- name: Unzip clients-javascript artifact |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if output variables from main workflow are available here? We could maybe have some automated ones set to avoid hardcoding paths
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have testsToDelete
and path
computed at the matrix level, idk if it can help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can pass the matrix but it would still require to write it for every language, might still be better than this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used them here also https://github.com/algolia/api-clients-automation/pull/689/files#diff-2137ec1d9b1926f1419ffdc8b47f8760b734131c88aad8396524673f13d7c62eR40 but it's not working
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually it's an array so it would be a pain to find the correct client with the correct name and then rm, it's easier like that
🧭 What and Why
Delete the files before applying the zip so that deleted files get commited too.
🧪 Test
CI